Skip to content

fix(appengine): removing obsolete symlink from java 8 on java 17 bundle services#10270

Open
Kef131 wants to merge 3 commits into
GoogleCloudPlatform:mainfrom
Kef131:fix-symlink-java8-newjavaversions
Open

fix(appengine): removing obsolete symlink from java 8 on java 17 bundle services#10270
Kef131 wants to merge 3 commits into
GoogleCloudPlatform:mainfrom
Kef131:fix-symlink-java8-newjavaversions

Conversation

@Kef131
Copy link
Copy Markdown

@Kef131 Kef131 commented May 19, 2026 •
edited
Loading

Description

Fixes #10242
Internal: b/496677589

This PR dereferences the symbolic links in the Java 17 App Engine bundled services module (appengine-java17-bundled-services) that previously pointed to the deleted/legacy appengine-java8 directory. By making these files self-contained, this module can run independently without relying on Java 8 code.

Additionally, this PR adds the missing JSTL dependency to the Java 17 pom.xml to resolve JSP runtime compilation errors (JasperException / ClassNotFoundException for guestbook_jsp) when running the development server locally with mvn appengine:run.

This PR dereferenced 32 Symbolic links on appengine-java17-bundled-services/datastore

Checklist

  • I have followed Sample Format Guide
  • pom.xml parent set to latest shared-configuration
  • Appropriate changes to README are included in PR
  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • Tests pass: mvn clean verify required
  • Lint passes: mvn -P lint checkstyle:check required
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Please merge this PR for me once it is approved

@product-auto-label product-auto-label Bot added api: appengine Issues related to the App Engine Admin API API. samples Issues that are directly related to samples. labels May 19, 2026
@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 19, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist[bot]
gemini-code-assist Bot reviewed May 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the App Engine Datastore guestbook application to the Java 17 runtime. The review identified a critical Cross-Site Scripting (XSS) vulnerability in the JSP file, a violation of the Post-Redirect-Get (PRG) pattern in the servlet, and a UX issue where the pagination link is displayed even when no further results are available.

I am having trouble creating individual review comments. Click here to see my feedback.

appengine-java17-bundled-services/datastore/src/main/webapp/guestbook.jsp (28)

security-high high

The user-provided content is rendered directly in the HTML without escaping. This creates a Cross-Site Scripting (XSS) vulnerability. Use <c:out> or fn:escapeXml to safely render user input.

        <c:out value="${greeting.content}"/><br>

appengine-java17-bundled-services/datastore/src/main/java/com/example/appengine/AbstractGuestbookServlet.java (55-56)

medium

After a successful doPost, the servlet forwards to the JSP instead of redirecting. This violates the Post-Redirect-Get (PRG) pattern, which can lead to duplicate form submissions if the user refreshes the page. Consider using a redirect after the greeting is appended.

    guestbook.appendGreeting(content);
    resp.sendRedirect(req.getRequestURI());

appengine-java17-bundled-services/datastore/src/main/java/com/example/appengine/ListPeopleServlet.java (88)

medium

The "Next page" link is always displayed, even if there are no more results to fetch. This can lead to a poor user experience where clicking the link results in an empty page. You should only display the link if the number of results returned is equal to the PAGE_SIZE.

    if (results.size() == PAGE_SIZE) {
      w.println("<a href='/people?cursor=" + cursorString + "'>Next page</a>");
    }

@Kef131 Kef131 mentioned this pull request May 20, 2026
Draft
11 tasks
@amcolin amcolin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 20, 2026
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 20, 2026
@Kef131
Copy link
Copy Markdown
Author

Kef131 commented May 21, 2026

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed May 21, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the Datastore sample application to App Engine Java 17 by replacing legacy references with actual source implementations. The changes include the addition of a jstl dependency and the implementation of various servlets and utility classes that demonstrate Datastore features such as CRUD operations, ancestor queries for strong consistency, projection queries, and pagination using cursors. The PR also includes necessary configuration files like web.xml and datastore-indexes.xml, along with a comprehensive suite of unit tests for entities, transactions, and metadata. I have no feedback to provide.

@Kef131 Kef131 self-assigned this May 21, 2026
@Kef131 Kef131 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 21, 2026
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 21, 2026
@Kef131 Kef131 marked this pull request as ready for review May 21, 2026 23:22
@Kef131 Kef131 requested review from a team and yoshi-approver as code owners May 21, 2026 23:22
@snippet-bot
Copy link
Copy Markdown

snippet-bot Bot commented May 21, 2026

Here is the summary of possible violations 😱

Details

There are 39 possible violations for not having product prefix.
There is a format violation for a region tag.

The end of the violation section. All the stuff below is FYI purposes only.

Here is the summary of changes.

You are about to add 63 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yoshi-approver yoshi-approver Awaiting requested review from yoshi-approver yoshi-approver is a code owner

1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@Kef131 Kef131

Labels

api: appengine Issues related to the App Engine Admin API API. samples Issues that are directly related to samples.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

GAE samples are pointing to deleted java8 code, we need to dereference from old version.

3 participants

@Kef131 @amcolin @kokoro-team